Skip to content

Unify the two branches in the consistency proof check - #149

Merged
davidben merged 3 commits into
mainfrom
unify-consistency
Oct 6, 2025
Merged

Unify the two branches in the consistency proof check#149
davidben merged 3 commits into
mainfrom
unify-consistency

Conversation

@davidben

@davidben davidben commented Oct 6, 2025

Copy link
Copy Markdown
Collaborator

Also replace prepending to the proof list with removing in the other branch. It makes it a little obvious how to implement this without making a copy of the proof. This should also hopefully make the relationship between inclusion and consistency proof evaluation clearer.

While I'm here, use '1.' for the steps so we don't have to renumber it as it changes.

(CC @nharper)

Also replace prepending to the proof list with removing in the other
branch. It makes it a little obvious how to implement this without
making a copy of the proof. This should also hopefully make the
relationship between inclusion and consistency proof evaluation clearer.

While I'm here, use '1.' for the steps so we don't have to renumber it
as it changes.

@lukevalenta lukevalenta left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice refactor! Just a couple comments.

4. Right-shift `sn` once more.
5. Compare `sn` to `0` and `r` to `root_hash`. If either is not equal, fail the proof verification. If all are equal, accept the proof.
2. Otherwise, run the following:
1. Set `fn` to `start`, `sn` to `end - 1`, and `tn` to `n - 1`.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I'm re-implementing this logic, I found a panic in the original version of my code when n = 0 due to an underflow. The fact that [start, end) is a subtree of the tree of size n implies that 0 <= start < end <= n so it must be the case that end > 0 and n > 0, but maybe we should add an explicit note about that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... how about we just make the function check the preconditions? It's fallible anyway, and makes it consistent with the inclusion proof version. Arguably it's redundant with the "type signature" but people will be sloppy.

Comment thread draft-davidben-tls-merkle-tree-certs.md
lukevalenta added a commit to cloudflare/azul that referenced this pull request Oct 6, 2025
lukevalenta added a commit to cloudflare/azul that referenced this pull request Oct 6, 2025
See ietf-plants-wg/merkle-tree-certs#149 for details.
As an added bonus, this removes a `clone()` of the proof.
@davidben
davidben merged commit ddafe78 into main Oct 6, 2025
2 checks passed
@davidben
davidben deleted the unify-consistency branch October 6, 2025 22:25
lukevalenta added a commit to cloudflare/azul that referenced this pull request Oct 7, 2025
See ietf-plants-wg/merkle-tree-certs#149 for details.
As an added bonus, this removes a `clone()` of the proof.
lukevalenta added a commit to cloudflare/azul that referenced this pull request Oct 7, 2025
See ietf-plants-wg/merkle-tree-certs#149 for details.
As an added bonus, this removes a `clone()` of the proof.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants